-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#12729: add row-major split for BERT #12804
Conversation
If @sjameelTT's recent changes to slice make it in first, I'll need to update some of the slice calls here to use end-exclusive indexing. |
tests/tt_eager/python_api_testing/unit_testing/misc/test_split_any_dim_rm.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you a codeowner under data_movement yet? I feel like you should add yourself there @tarafdarTT thoughts?
tests/tt_eager/python_api_testing/unit_testing/misc/test_split_any_dim_rm.py
Outdated
Show resolved
Hide resolved
hey @jaykru-tt , I'm a little confused with the description. I'm asking because I thought for Bert split we were going to use Tile layout but your description alludes to Bert requiring RM layout. |
tests/tt_eager/python_api_testing/unit_testing/misc/test_split_any_dim_rm.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great stuff @jaykru-tt
post-commit passed, lfg, merging this |
Ticket
#12729
Problem description
The split operation didn't support row major tensors or even the shape [1,256,2] required by BERT when in tiled format.
What's changed
This commit introduces a row major support for the split operation, additionally allowing for n-way splits on any dimension and splits on tensors of rank != 4. I've also added a few test cases, but these only cover 2-way splits on shapes similar to the shape required by BERT.
Checklist